Skip to content

Conversation

@kparzysz
Copy link
Contributor

@kparzysz kparzysz commented May 4, 2025

ConstantBase has a constructor that takes a value of any type as an input: template ConstantBase(const T &). A derived type Constant is a member of many Expr classes (as an alternative in the member variant).

When trying (erroneously) to create Expr from a wrong input, if the specific instance of Expr contains Constant, it's that constructor that will be instantiated, leading to cryptic and unexpected errors.

Eliminate the constructor from overload for invalid input values to help produce more meaningful diagnostics.

ConstantBase has a constructor that takes a value of any type as an
input: template <typename T> ConstantBase(const T &). A derived type
Constant<T> is a member of many Expr<T> classes (as an alternative in
the member variant).

When trying (erroneously) to create Expr<T> from a wrong input, if the
specific instance of Expr<T> contains Constant<T>, it's that constructor
that will be instantiated, leading to cryptic and unexpected errors.

Eliminate the constructor from overload for invalid input values to help
produce more meaningful diagnostics.
@kparzysz kparzysz requested a review from klausler May 4, 2025 16:12
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:semantics labels May 4, 2025
@llvmbot
Copy link
Member

llvmbot commented May 4, 2025

@llvm/pr-subscribers-flang-semantics

Author: Krzysztof Parzyszek (kparzysz)

Changes

ConstantBase has a constructor that takes a value of any type as an input: template <typename T> ConstantBase(const T &). A derived type Constant<T> is a member of many Expr<T> classes (as an alternative in the member variant).

When trying (erroneously) to create Expr<T> from a wrong input, if the specific instance of Expr<T> contains Constant<T>, it's that constructor that will be instantiated, leading to cryptic and unexpected errors.

Eliminate the constructor from overload for invalid input values to help produce more meaningful diagnostics.


Full diff: https://github.com/llvm/llvm-project/pull/138456.diff

1 Files Affected:

  • (modified) flang/include/flang/Evaluate/constant.h (+5-1)
diff --git a/flang/include/flang/Evaluate/constant.h b/flang/include/flang/Evaluate/constant.h
index 6fc22e3b86aa2..d4c6601c37bca 100644
--- a/flang/include/flang/Evaluate/constant.h
+++ b/flang/include/flang/Evaluate/constant.h
@@ -110,8 +110,12 @@ class ConstantBase : public ConstantBounds {
   using Result = RESULT;
   using Element = ELEMENT;
 
-  template <typename A>
+  // Constructor for creating ConstantBase from an actual value (i.e.
+  // literals, etc.)
+  template <typename A,
+      typename = std::enable_if_t<std::is_convertible_v<A, Element>>>
   ConstantBase(const A &x, Result res = Result{}) : result_{res}, values_{x} {}
+
   ConstantBase(ELEMENT &&x, Result res = Result{})
       : result_{res}, values_{std::move(x)} {}
   ConstantBase(

@kparzysz kparzysz merged commit 304c7a8 into llvm:main May 6, 2025
14 checks passed
@kparzysz kparzysz deleted the users/kparzysz/constant-base branch May 6, 2025 17:00
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
…8456)

ConstantBase has a constructor that takes a value of any type as an
input: template <typename T> ConstantBase(const T &). A derived type
Constant<T> is a member of many Expr<T> classes (as an alternative in
the member variant).

When trying (erroneously) to create Expr<T> from a wrong input, if the
specific instance of Expr<T> contains Constant<T>, it's that constructor
that will be instantiated, leading to cryptic and confusing errors.

Eliminate the constructor from overload for invalid input values to help
produce more meaningful diagnostics.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:semantics flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants